-
Notifications
You must be signed in to change notification settings - Fork 5.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add android announcement banner #7551
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Codecov Report
Flags with carried forward coverage won't be shown. Click here to find out more. |
1 flaky test on run #15406 ↗︎Details:
|
Test | Artifacts | |
---|---|---|
swap flow logging > completes two swaps and verifies the TTS logging for the first, plus all intermediate steps along the way |
Screenshots
|
Review all test suite changes for PR #7551 ↗︎
@@ -1,105 +0,0 @@ | |||
import { Trans } from '@lingui/macro' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I think removal of the Base banner would ideally happen in a separate PR with its migration
} | ||
` | ||
export const StyledQrCode = styled.img` | ||
padding: 2px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
confirming 2px and not 4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes - qr code was too small so phil wanted to decrease padding
display: flex; | ||
flex-direction: column; | ||
align-items: flex-start; | ||
gap: 2px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
confirming 2px here and not 4px
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a few nits but overall this lgtm
lfg 🚢
Adding new Android announcement banner. Approved by design 👍
Will delete hideBaseWalletBanner redux state var via migration in another PR
Description
Slack thread: https://uniswapteam.slack.com/archives/C060WANHWBB/p1699369785721189
Figma: https://www.figma.com/file/4DJrdsx1lPm5u409xyKDZj/%F0%9F%8D%84-%F0%9F%95%B8%EF%B8%8F-Spore---Web?type=design&node-id=14716-56627&mode=design&t=L0pXaQoI6uuAaq8N-0
Screen capture